fix: 2FA login always shows generic "bad credentials" error#9
Conversation
Agent-Logs-Url: https://github.com/ninocss/UntisPlus/sessions/20db486e-0a71-4998-80ba-20d37545d7b7 Co-authored-by: ninocss <108231535+ninocss@users.noreply.github.com>
…ed as invalid OTP Agent-Logs-Url: https://github.com/ninocss/UntisPlus/sessions/20db486e-0a71-4998-80ba-20d37545d7b7 Co-authored-by: ninocss <108231535+ninocss@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adjusts WebUntis 2FA error handling so OTP-related login failures don’t fall back to the generic “check your credentials” message when the server responds with a generic error.
Changes:
- Updates
_authenticateUntisto treat any JSON-RPC error asotpInvalidwhen an OTP was provided. - Adds an explanatory comment describing the intended UI behavior for 2FA failures.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Treat any server error as an invalid OTP when a code was provided, so | ||
| // the caller can show the 2FA-specific error instead of the generic | ||
| // "check your credentials" message. | ||
| final invalidOtp = | ||
| combined.contains('invalid otp') || | ||
| combined.contains('invalid verification') || | ||
| combined.contains('wrong otp') || | ||
| combined.contains('otp invalid') || | ||
| (contains2faHint && otpCode != null && otpCode.isNotEmpty); | ||
| (otpCode != null && otpCode.isNotEmpty); | ||
| if (invalidOtp) { |
There was a problem hiding this comment.
invalidOtp now becomes true for any JSON-RPC error whenever a non-empty OTP was sent. This will classify unrelated server-side/auth errors (e.g., wrong username/password, invalid school/server, temporary backend failures) as “OTP invalid”, which can mislead users and mask actionable errors. Consider restricting this fallback to clearly auth-related failures (e.g., specific error codes/messages like "bad credentials") or returning a distinct flag so the UI can show a non-OTP error when appropriate.
| final invalidOtp = | ||
| combined.contains('invalid otp') || | ||
| combined.contains('invalid verification') || | ||
| combined.contains('wrong otp') || | ||
| combined.contains('otp invalid') || | ||
| (contains2faHint && otpCode != null && otpCode.isNotEmpty); | ||
| (otpCode != null && otpCode.isNotEmpty); |
There was a problem hiding this comment.
With the new (otpCode != null && otpCode.isNotEmpty) clause, the preceding string checks in invalidOtp are redundant for the OTP-present path (the expression will always be true). If the intent is “any error when OTP is provided”, this can be simplified to improve readability and avoid implying that keyword matching still matters in that branch.
When a WebUntis server returns a generic error (e.g.
"bad credentials") in response to anauthenticaterequest that includes an OTP, the app showed the generic "Prüfe deine Daten" toast instead of the 2FA-specific "Code ungültig" error — leaving the user no indication that the OTP was the problem.Root cause
_authenticateUntisonly returned{otpInvalid: true}when the server's error message contained explicit OTP/2FA keywords ("otp","2fa","mfa", …). A generic"bad credentials"response fell through toreturn null, which_handleLoginmaps tologinFailed.Change
Simplified the
invalidOtpcondition in_authenticateUntis(lib/main.dart) to treat any server error as an invalid OTP when an OTP was provided:This ensures the caller always receives
{otpInvalid: true}in that path, keeping the 2FA input visible and surfacing the correct error message.